Skip to content

Add official EVM Block test data and execution#35

Open
ceyonur wants to merge 40 commits intomainfrom
ceyonur/eth-tests
Open

Add official EVM Block test data and execution#35
ceyonur wants to merge 40 commits intomainfrom
ceyonur/eth-tests

Conversation

@ceyonur
Copy link
Contributor

@ceyonur ceyonur commented Dec 4, 2025

This PR adds the test execution (from https://github.com/ava-labs/libevm/tree/main/tests) and testdata as a submodule from the repository https://github.com/ethereum/tests (at fa51c5c164 as same commit in libevm@main). It only executes BlockchainTests and LegacyTests which are inserting test blockchains (in json files) with SAE's own blockbuilder/chain (through SUT) and then executes with saexec. Tests are passing but with few limitations and hacks (see saexec/ethtests/block_test.go and comments for that file) :

1- GeneralStateTest are too slow tried to run them but CI fails see comment
2- We cannot run InvalidBlocks test because they're asserting either that block insertion is failing (which we don't have any verification yet), or they fail at execution (which we don't allow). We can revisit them once we add the verification.
3- There are some tests like bcForkStressTest and bcMultiChainTest testing multi-chains and forks with non-linear blocks and multiple possible chains. We currently don't support this in test chain builders. So these tests are skipped.
4- We don't have any execution pre-checks yet (like MaxCodeSize and intrinsic gas checks) so some tests are failing with a FAIL log in saexec. Once we have those checks we can execute those tests.
5- SAE has it's own baseFee applied to the header just before the execution, which means header's baseFee won't directly applied to the state where EVM tests expect it to be applied as it's; thus failing some state tests like balances, rewards etc. This is currently handled by faking the gas clock in insertWithHeaderBaseFee

@ceyonur ceyonur changed the base branch from main to ceyonur/init-eth-tests December 4, 2025 10:42
@ceyonur ceyonur changed the base branch from ceyonur/init-eth-tests to main December 16, 2025 15:43
// using 4.6 TGas
bt.skipLoad(`.*randomStatetest94.json.*`)

// TODO(cey): We cannot run invalid blocks, since we don't currently have pre-insert checks.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of this file is a copy-pasta from libevm. Here we specifically skip some tests that's not currently supported.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of this file is a copy-pasta from libevm

Why? The EEST documentation states that they are...

test fixtures (JSON) which can be consumed by any execution client to verify their implementation

...meaning that they're not even language-specific, let alone related to an implementation. I very strongly believe that we should be using these as inputs in an SAE-specific manner instead of trying to shoe-horn SAE into geth.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were not skipped due to a forced integration between geth and sae, but rather we do not have any means of invalidating a block insertion (due to lack of validations in SAE). Maybe we can now revisit this as we have implemented a few of them, but I rather to have that in a separate PR.

Copy link
Contributor Author

@ceyonur ceyonur Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from PR description:

2- We cannot run InvalidBlocks test because they're asserting either that block insertion is failing (which we don't have any verification yet), or they fail at execution (which we don't allow). We can revisit them once we add the verification.


// TestExecutionSpecBlocktests runs the test fixtures from execution-spec-tests.
func TestExecutionSpecBlocktests(t *testing.T) {
if !common.FileExist(executionSpecBlockchainTestDir) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is actually an empty file, so we don't run this test.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then why is it included in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an outdated comment I forgot to remove. We now download those spec fixtures in CI (fixtures_develop.tar.gz)

@ceyonur ceyonur changed the title Ceyonur/eth tests Add go-ethereum/tests testdata Dec 17, 2025
@ceyonur ceyonur changed the title Add go-ethereum/tests testdata Add official EVM testdata Dec 17, 2025
@ceyonur ceyonur self-assigned this Dec 17, 2025
@ceyonur ceyonur linked an issue Dec 17, 2025 that may be closed by this pull request
@ceyonur ceyonur changed the title Add official EVM testdata Add official EVM Block test data and execution Dec 17, 2025
return validBlocks, nil
}

func insertWithHeaderBaseFee(tb testing.TB, sut *SUT, bs types.Blocks) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the hack to fake the gas clock in parent by setting excess to calculated header.BaseFee

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a useful primitive so please abstract blockstest.WithFakeBaseFee(testing.TB, *blocks.Block, [other necessary args]) *blocks.Block.

@ceyonur ceyonur marked this pull request as ready for review January 2, 2026 13:34
Comment on lines 111 to 117
gen := conf.genesisSpec
if gen == nil {
gen = &core.Genesis{
Config: config,
Timestamp: conf.timestamp,
Alloc: alloc,
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default value should instead be set on conf and options.ApplyTo() might then override it. Placing defaults in the original allows options to use them; even if not something that's being done here, it's best practice.

EXECUTION_SPEC_TESTS_FILE: fixtures_develop.tar.gz
steps:
- uses: actions/checkout@v4
with:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put all of these in a new job, perhaps go_spec_tests, and add it to the needs of go (to make CI require it). The rationale is that the other tests are more likely to be the ones failing and we want to give that feedback as quickly as possible—placing these in a new job makes it run in parallel.

}

// Insert adds a block to the chain.
func (cb *ChainBuilder) Insert(block *blocks.Block) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NewChainBuilder() (for genesis) and ChainBuilder.NewBlock() should use Insert() so all paths to insert a block are equivalent.

Comment on lines 89 to 92
func (cb *ChainBuilder) Insert(block *blocks.Block) {
cb.chain = append(cb.chain, block)
cb.blocksByHash.Store(block.Hash(), block)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func (cb *ChainBuilder) Insert(block *blocks.Block) {
cb.chain = append(cb.chain, block)
cb.blocksByHash.Store(block.Hash(), block)
}
func (cb *ChainBuilder) Insert(tb testing.TB, b *blocks.Block) {
tb.Helper()
if len(cb.chain) > 0 {
p := cb.Last()
require.Equalf(tb, p.Height()+1, b.Height(), "%T.Insert() non-incrementing block height", cb)
require.Equalf(tb, p.Hash(), b.ParentHash(), "%T.Insert() with mismatched parent hash", cb)
}
cb.chain = append(cb.chain, b)
cb.blocksByHash.Store(b.Hash(), b)
}

Otherwise we risk breaking internal invariants.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial thought was we should not fail any tests in those cases as it's intentional to insert invalid blocks (as stated here).

However it seems this should be fine. I checked that in upstream by putting a panic here and seems there is no such case https://github.com/ava-labs/libevm/blob/main/core/blockchain.go#L1532-L1533.

Anyway this change should fine for this PR as we do not enable InvalidBlocks tests yet.

Copy link
Contributor Author

@ceyonur ceyonur Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However it seems this should be fine. I checked that in upstream by putting a panic here and seems there is no such case https://github.com/ava-labs/libevm/blob/main/core/blockchain.go#L1532-L1533.

On the second thought: This only checks if the given chain is ordered and linked, but it does not really check anything if it's a chain of single block (in a case we're inserting a single block). So tests actually expect a returned error (not failed test) in such cases.

As I said it won't fail in this PR, but it seems we need to return errors when we actually insert blocks (either in this blockstest package or through a prod-level chain builder)


// GetHashAtHeight returns the hash of the block at the given height, and a flag indicating if it was found.
// If the height is greater than the number of blocks in the chain, it returns an empty hash and false.
func (cb *ChainBuilder) GetHashAtHeight(num uint64) (common.Hash, bool) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only ever used to pass the hash into ChainBuilder.GetBlock() so could just be BlockByNumber(testing.TB, uint64) *blocks.Block and similarly GetNumberByHash() could just be BlockByHash(testing.TB, common.Hash) *blocks.Block.

Since they don't need to conform to any particular signature (unlike GetBlock(), which is a blocks.Source) they can do the test failures internally if the block doesn't exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed them but omitted the TB param, since we're calling these from the readerAdapter from consensus.ChainHeaderReader , which we cannot change the signature and take TB. We can set a TB for the readerAdapter when we create them but not sure if it's a good idea to carry it around.

// using 4.6 TGas
bt.skipLoad(`.*randomStatetest94.json.*`)

// TODO(cey): We cannot run invalid blocks, since we don't currently have pre-insert checks.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of this file is a copy-pasta from libevm

Why? The EEST documentation states that they are...

test fixtures (JSON) which can be consumed by any execution client to verify their implementation

...meaning that they're not even language-specific, let alone related to an implementation. I very strongly believe that we should be using these as inputs in an SAE-specific manner instead of trying to shoe-horn SAE into geth.

opts = append(opts, withGenesisSpec(gspec))

// Wrap the original engine within the beacon-engine
engine := beacon.New(ethash.NewFaker())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Invoking the Beacon chain is IMO strongly suggestive of this not being an appropriate approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed, this is why we require #72. Otherwise we will always need to comply with Ethereum rules and specs/fixtures for testing these.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding from stand-up yesterday was that #72 was to allow for testing of coreth/subnet-evm variants, but not for SAE/libevm as they would just use upstream spec tests. What I mean (here too) is that the JSON can be parsed to build and run blocks directly against the saexec.Executor.

Copy link
Contributor Author

@ceyonur ceyonur Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While #72 will be crucial for coreth/subnet-evm, bu SAE is also not 1-1 compliant with those JSON fixtures (baseFee, missing blob handling, beacon etc). We're currently patching those through upstream engines and before/after hooks with upstream-like handlings (difficulty, coinbase, beacon etc). In fact we should generate those fixtures without running any of these (and skip some specs/fixtures like beacon/difficulty/blobs i.e stuff we don't really support/have)

I'm not sure what you mean by parsing JSON. We do parse them, create blocks from those JSONs and run blocks directly against the executor (here). If you mean modifying JSON that will be generating fixtures (as described in #72).


// TestExecutionSpecBlocktests runs the test fixtures from execution-spec-tests.
func TestExecutionSpecBlocktests(t *testing.T) {
if !common.FileExist(executionSpecBlockchainTestDir) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then why is it included in this PR?

return validBlocks, nil
}

func insertWithHeaderBaseFee(tb testing.TB, sut *SUT, bs types.Blocks) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a useful primitive so please abstract blockstest.WithFakeBaseFee(testing.TB, *blocks.Block, [other necessary args]) *blocks.Block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Run official EVM tests against SAE

2 participants